peep.c: Only remove expected empty if/else block structures #23512
Conversation
The expected OP tree structures for empty blocks fit for removal was:
```
+-- stub
```
or
```
+-- scope
|
+-- stub
```
GH Perl#23505 showed that `{ BEGIN {} }` blocks will also generate the likes of:
```
+--scope
|
+--stub
|
+--null (ex-nextstate)
```
This commit does the minimal possible to fix this bug - the peephole optimizer
now checks for the expected cases and ignores anything else.
Future commits may build on this to remove this additional type of structure.
Additional tests have been added now and give a useful structure for future
commit tests to fit into.
|
Rather than handling the However, initial work showed that I'll work on this a bit more in the background. |
By this, do you mean this PR is still in progress (and should be marked draft), or that you'll do that separately in a different PR? I.e. should we still review this one? |
| op_sibling_splice(o, cLOGOP->op_first, 1, NULL); | ||
| op_free(stub); | ||
| op_free(trueop); |
There was a problem hiding this comment.
I had forgotten that op_free will free kids, so this should be just op_free(trueop) before being merged.
|
Apologies for the vagueness of that first comment!
Basically, this PR should fix #23505 as-is but I then intend to build on it later in this dev cycle. |
@richardleach can we move forward with this pull request? Who should be reviewing it? I ask because I anticipate other BBC breakage for distributions which use Test-Inter in their test suites, e.g., Date-Manip. Thanks. |
#23367 expected that empty
OP_COND_EXPRbranches would produce one of the following OP tree structures fit for removal:
or
GH #23505 showed that
{ BEGIN {} }blocks will also generate the likes of:This commit does the minimal possible to fix this bug - the peephole optimizer
now checks for the expected cases and ignores anything else.
Future commits may extend the optimization to remove this additional type of
structure. Additional tests have been added now and give a useful structure for
future commit tests to fit into.